perf!: yoke zero-copy node decoding and Jid Server enum - #513

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/client.rs`:
- Around line 3265-3267: The call here passes a &wacore_binary::NodeRef but
unified_session.update_server_time_offset expects a &Node, so either change
UnifiedSessionManager::update_server_time_offset to accept
&wacore_binary::NodeRef<'_> (update its signature and usages) or convert the
NodeRef to the owned Node before calling
unified_session.update_server_time_offset (e.g., obtain an owned Node via the
appropriate conversion/clone method on NodeRef and pass a &Node). Target
symbols: update_server_time_offset (in this file),
unified_session.update_server_time_offset,
UnifiedSessionManager::update_server_time_offset, and wacore_binary::NodeRef.
- Around line 1687-1693: The failure comes from a mismatch between should_ack's
signature and how it's used: should_ack currently takes
&wacore_binary::NodeRef<'_> (function should_ack) but callers/tests pass &Node;
either change should_ack to accept &wacore_binary::Node (if that API is easier)
or update the callers/tests to pass a NodeRef by converting the Node to a
NodeRef (e.g. node.as_ref() / node.to_ref() / Node::as_ref() depending on the
crate API) and ensure attribute checks use the correct accessor for NodeRef
(e.g. get_attr(...) -> Option<&ValueRef> or attr(...) as provided by the
library); pick one consistent approach and update all usages/tests to match
should_ack and the Node/NodeRef attribute accessor.

In `@src/client/device_registry.rs`:
- Around line 307-313: The code silently downgrades failed Server conversion to
Server::Pn causing LID sessions to be skipped; replace the
try_from(...).unwrap_or(Server::Pn) fallback by iterating the Server enum
variants explicitly (so both LID and PN are handled) when building the Jid
inside the loops; locate the loop using variables servers, lookup.all_keys(),
device_ids and the conversion call wacore_binary::jid::Server::try_from(srv) /
Server::Pn and change it to enumerate the actual Server variants (or otherwise
obtain both relevant Server variants) before calling Jid::new and setting
jid.device so delete_sessions_for_devices() will run for each server variant
rather than masking conversion failures.

In `@src/handlers/chatstate.rs`:
- Around line 57-58: The handler is passing an Arc<OwnedNodeRef> to
ChatstateStanza::parse which expects &Node; change the call to pass the
converted node by invoking to_owned_node() on the Arc (e.g., let owned =
node.to_owned_node(); then call ChatstateStanza::parse(&owned)) so parse
receives a &Node instead of &Arc<OwnedNodeRef>.

In `@src/handlers/ib.rs`:
- Around line 29-30: The helper handle_ib_impl must be migrated from taking
&Node to the zero-copy type: change its signature to accept
Arc<wacore_binary::OwnedNodeRef> (e.g. async fn handle_ib_impl(client:
Arc<Client>, node: Arc<wacore_binary::OwnedNodeRef>) -> ...), update any
internal code in handle_ib_impl that references Node to use
wacore_binary::OwnedNodeRef, and update the call sites in this file (currently
calling handle_ib_impl(client, &node)) to pass the Arc (e.g.
handle_ib_impl(client, node.clone()) or handle_ib_impl(client, node) as
appropriate). Also adjust imports to bring wacore_binary::OwnedNodeRef into
scope.

In `@src/handlers/notification.rs`:
- Around line 37-39: The handler method handle currently passes an
Arc<wacore_binary::OwnedNodeRef> into handle_notification_impl which expects a
&Node; convert the yoke-backed stanza to an owned Node at the boundary by
calling the to_owned_node() method on the OwnedNodeRef before awaiting
handle_notification_impl. Locate the handle function and change the argument
from &node to a temporary owned_node produced via node.to_owned_node() (or
similar) and pass &owned_node into handle_notification_impl so types match.

In `@src/handlers/receipt.rs`:
- Around line 23-24: The handler currently passes
Arc<wacore_binary::OwnedNodeRef> (named node) into client.handle_receipt, but
Client::handle_receipt expects Arc<Node>, causing the E0308; fix by converting
the OwnedNodeRef to the expected Arc<Node> before calling client.handle_receipt
(or alternatively change Client::handle_receipt to accept
Arc<wacore_binary::OwnedNodeRef> to finish the migration). Locate the async fn
handle in src/handlers/receipt.rs and either (a) transform node into an
Arc<Node> (using the appropriate conversion/constructor provided by
wacore_binary, e.g., a from/into/to_node helper) and pass that to
client.handle_receipt, or (b) update the signature of Client::handle_receipt to
accept Arc<wacore_binary::OwnedNodeRef> and adjust its implementation
accordingly so types match.

In `@src/handlers/router.rs`:
- Around line 49-56: Tests and MockHandler need to be updated to use the new
zero-copy OwnedNodeRef type: change the MockHandler impl and test fixtures that
currently accept Arc<Node> to accept Arc<wacore_binary::OwnedNodeRef> (so they
match dispatch signature and handlers.get(...).handle(client, node,
cancelled).await), and update the dispatch tests to construct and pass
Arc<OwnedNodeRef> instances instead of Arc<Node> when calling Router::dispatch;
ensure any pattern matching or field access in MockHandler uses node.get() like
the production code.

In `@src/handlers/unimplemented.rs`:
- Around line 46-47: In handle (async fn handle) replace direct access to
node.tag with the owned node's borrowed NodeRef: call node.get() to obtain the
NodeRef, then read the tag via node_ref.tag.as_ref() and pass that &str into
client.handle_unimplemented(...). Concretely, inside handle use let nr =
node.get(); then await client.handle_unimplemented(nr.tag.as_ref()).await so you
respect OwnedNodeRef's API and convert the Cow<str> to &str.

In `@wacore/binary/src/decoder.rs`:
- Around line 111-114: The code currently coerces an invalid JID server into
Server::Pn; instead, when crate::jid::Server::try_from(server_str.as_ref())
fails (i.e., server_str is missing/unknown), return or propagate a decoding
error rather than defaulting to Server::Pn. Locate the server parsing in
decoder.rs (the read_value_as_string()/server_str and Server::try_from usage)
and replace the unwrap_or(...Server::Pn) path with proper error handling: map
the try_from failure into the decoder's Result error (or use ? to propagate) so
malformed JID_PAIR server values fail fast.

In `@wacore/binary/src/jid.rs`:
- Around line 296-298: The is_ad method currently checks server() against
Server::Pn, Server::Lid, and Server::Hosted but omits Server::HostedLid, causing
`@hosted.lid` device JIDs to be misclassified; update the match in fn is_ad(&self)
-> bool (and any similar checks) to include Server::HostedLid alongside Pn, Lid,
and Hosted so hosted-LID device JIDs are recognized as AD.

In `@wacore/src/iq/privacy.rs`:
- Line 638: The test fixture currently uses a phone-like PN JID via pn_jid:
Some(Jid::new("5511999999999", Server::Pn)) which appears real; update the
fixture to use an obviously fictitious/reserved identifier (e.g., a clearly fake
phone/JID string) when constructing the pn_jid with Jid::new and keep Server::Pn
unchanged so tests avoid real PII—locate the pn_jid field in
wacore/src/iq/privacy.rs and replace the literal with a clearly fake value.

---

Outside diff comments:
In `@src/client.rs`:
- Around line 3100-3105: In handle_connect_failure replace the call to
node.attrs() with node.attr_parser() (same fix as handle_stream_error): obtain
the attribute parser via let mut attrs = node.attr_parser(); and then use
attrs.optional_u64("reason").unwrap_or(0) as i32 to read the reason code,
ensuring the parser API is used instead of the non-existent attrs() method.
- Around line 1412-1416: The code attempts to access node.tag on an OwnedNodeRef
returned by decrypt_frame, causing a compilation error; update the matches call
to use node.get().tag (i.e., call .get() on the OwnedNodeRef before inspecting
tag) so process_inline is computed from node.get().tag, ensuring you dereference
the OwnedNodeRef returned by decrypt_frame when checking tags like "success" |
"failure" | "stream:error" | "message" | "ib".
- Around line 1926-1941: The code attempts to index node.attrs with get("lid")
which fails because NodeRef.attrs is a slice of (Cow<str>, ValueRef) pairs;
change the extraction in the lid_from_server block to use the NodeRef API or
iterate the slice: call node.get_attr("lid") if available or iterate node.attrs
to find the tuple whose 0 equals "lid", then convert its ValueRef to_jid() as
before; update the lid_from_server match to use that retrieved attribute (refer
to symbols: node.attrs, node.get_attr, lid_from_server,
self.update_server_time_offset) and preserve the same logging paths when
attribute is missing or parsing fails.
- Around line 1632-1651: The code currently passes an Arc<OwnedNodeRef>
(variable node) to places expecting Arc<Node>, causing type mismatches; update
the types to accept OwnedNodeRef to preserve zero-copy: change Event::RawNode
variant, the NodeWaiter type and resolve_node_waiters(&node) signature to accept
Arc<OwnedNodeRef> (or NodeRef/OwnedNodeRef as used in your codebase), update
NodeFilter::matches to take a NodeRef/OwnedNodeRef instead of Arc<Node>, and
update maybe_deferred_ack and send_ack_for signatures/usages to accept
Arc<OwnedNodeRef> (converting to owned Node only where strictly necessary).
Ensure all call sites that dispatch Event::RawNode, invoke resolve_node_waiters,
call NodeFilter::matches, and call maybe_deferred_ack/send_ack_for are updated
accordingly to the new OwnedNodeRef-based types.
- Around line 2982-2997: The compile error occurs because wacore_binary::NodeRef
has no attrs() method; update handle_stream_error to use node.attr_parser() (or
access the attr_parser field) instead: replace let mut attrs = node.attrs() with
let mut parser = node.attr_parser(); then get the code via
parser.optional_string("code").as_deref().unwrap_or(""); similarly, when mapping
the "conflict" child, call
n.attr_parser().optional_string("type").as_deref().unwrap_or("").to_string() so
all attribute parsing uses AttrParser methods instead of attrs(); keep existing
variable names (code_cow/code and conflict_type) and behavior otherwise.
- Around line 3698-3731: The tests call client.should_ack(&receipt_node) and
&notification_node but should_ack now expects &NodeRef; update the test to
create OwnedNodeRef instances from the existing Node values (marshal/encode each
Node into bytes and construct an OwnedNodeRef), then call
client.should_ack(owned_ref.get()) for both receipt and notification nodes and
assert the results as before; reference symbols: should_ack, OwnedNodeRef,
NodeRef, client, receipt_node, notification_node.
- Around line 3157-3178: In handle_iq replace the incorrect attrs accessors: use
node.get_attr("type") and node.get_attr("xmlns") instead of node.attrs.get(...),
and replace node.attrs() with node.attr_parser() when building the parser used
to extract from_jid and id (so keep build_pong(from_jid.to_string(),
id.as_deref()) and send_node usage unchanged). For the pair::handle_iq call
either convert the NodeRef into an owned Node before calling
pair::handle_iq(self, owned_node).await or update the pair::handle_iq signature
to accept &wacore_binary::NodeRef<'_> so the call matches; ensure the chosen
approach is applied consistently in this module and the pair module (referencing
handle_iq, build_pong, attr_parser, and pair::handle_iq).
- Around line 4887-4959: Tests call client.process_node(Arc::new(node)) but
process_node now expects Arc<OwnedNodeRef>, so update the tests (e.g., in
test_ib_thread_metadata_does_not_end_sync,
test_ib_edge_routing_does_not_end_sync, test_ib_dirty_does_not_end_sync,
test_ib_offline_child_ends_sync) to convert the Node built by NodeBuilder into
an OwnedNodeRef before wrapping in Arc; for example, construct the Node with
NodeBuilder, convert it to an OwnedNodeRef (using the crate’s conversion API
such as OwnedNodeRef::from(node) or the provided Node->OwnedNodeRef
constructor), then call client.process_node(Arc::new(owned_node_ref)). Ensure
references to process_node, OwnedNodeRef, NodeBuilder, and
create_offline_sync_test_client are updated accordingly.
- Around line 3070-3078: Replace uses of DisplayableNode(node) with
DisplayableNodeRef(node) because node is a &NodeRef; also ensure StreamError.raw
is given an owned Node by converting the NodeRef (e.g., Some(node.to_owned()) or
the appropriate to_owned/clone API) instead of passing the NodeRef directly.
Update the block creating crate::types::events::StreamError (and the other
similar sites using DisplayableNode(node)) to use DisplayableNodeRef and to pass
raw: Some(node.to_owned()) so the type matches Option<Node>.

In `@src/handlers/iq.rs`:
- Around line 26-34: The handler is treating Arc<OwnedNodeRef> as a Node/NodeRef
causing type errors; explicitly dereference the Arc<OwnedNodeRef> to a &NodeRef
before calling helper APIs. Inside handle(), bind a reference like let node_ref
= node.as_ref() or &*node and then call client.handle_iq(node_ref). Use
node_ref.attrs.get(...) for attributes and pass node_ref to DisplayableNode (or
use DisplayableNodeRef(node_ref) as suggested) so all three places (handle_iq,
attrs.get, DisplayableNode) receive the correct referenced type.

In `@src/handlers/message.rs`:
- Around line 32-113: The handler mixes OwnedNodeRef and Arc<Node>: replace
direct calls to node.attrs() with node.get().attrs() to access the inner
NodeRef, and when enqueuing convert the OwnedNodeRef into the queue's Arc<Node>
type (e.g., wrap the owned node: Arc::new(node.to_owned_node()) or whatever
conversion method exists) before calling tx.try_send; update the code paths
around handle, node.get(), attrs(), message_queues and tx.try_send to perform
these swaps so types align.

In `@src/handlers/presence.rs`:
- Around line 24-40: The handler uses Arc<wacore_binary::OwnedNodeRef>
(parameter name node) as if it exposes attrs directly, causing E0609; change
accesses to call node.get() to obtain the borrowed NodeRef and then use NodeRef
methods (e.g., get_attr("from") / get_attr("type") / get_attr("last") or access
the .attrs on the borrowed NodeRef) when parsing from_jid, unavailable, and
last_seen inside async fn handle; update all occurrences referencing node.attrs
to use node.get().get_attr(...) (or node.get().attrs) and adjust the last_seen
parsing to parse the string from the borrowed attribute value accordingly.

In `@wacore/binary/src/decoder.rs`:
- Around line 131-144: The decoder currently rejects unknown agent values for
AD_JID, breaking encode/decode symmetry; change the fallback so unmapped agent
bytes are treated as PN (preserving agent-coded PN AD_JID) instead of returning
an error. In the match on `agent` (the AD_JID domain type parsing) return
crate::jid::Server::Pn for the default/unknown-arm (or otherwise accept and map
unmapped `agent` values to `Server::Pn`) so values emitted by
`server_to_domain_type()` round-trip instead of erroring.

In `@wacore/binary/src/jid.rs`:
- Around line 705-710: The Display implementations for JID append ".{agent}"
when self.agent > 0 but currently skip only Server::Pn, Server::Lid, and
Server::Hosted; update the condition to also skip Server::HostedLid so the agent
is not appended for hosted-lid addresses (preventing outputs like
user.<agent>:<device>@hosted.lid); locate the branches that check
matches!(self.server, Server::Pn | Server::Lid | Server::Hosted) in the Display
impl(s) (the blocks that call write!(f, ".{}", self.agent)?), add
Server::HostedLid to that matches! list in both places (the other occurrence
around the 723-731 range) so hosted-lid follows the same suppression logic.
- Around line 652-685: The early branch that handles dotted-agent when server !=
DEFAULT_USER_SERVER && server != HIDDEN_USER_SERVER (the if let Some((u,
last_part)) = user.rsplit_once('.') && let Ok(num_val) =
last_part.parse::<u16>() block) currently assigns agent = num_val as u8 without
range-checking and can truncate values; add the same bounds check used in the
later fallback branch (if num_val > u8::MAX as u16 { return
Err(JidError::InvalidFormat(format!("Agent component out of range:
{num_val}"))); }) before casting, and keep the assignments user = u and agent =
num_val as u8 only after the check.

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/client.rs`:
- Around line 3182-3184: The code currently clones every incoming Node via
node.to_owned() before calling pair::handle_iq, which is expensive on the hot IQ
path; either change pair::handle_iq to accept a non-owning NodeRef/OwnedNodeRef
(e.g., update pair::handle_iq signature to take &NodeRef or OwnedNodeRef and
adjust pair.rs to work with that) so no clone is required for non-pair stanzas,
or add a cheap pre-check (inspect NodeRef metadata such as node.tag/name,
namespace, or presence of a known pair-specific child/attribute) before calling
node.to_owned() and only materialize the owned Node when the pre-check indicates
the stanza might be for pairing. Ensure you update calls and tests that invoke
pair::handle_iq to match the new signature if you choose the first option.
- Around line 4885-4892: The test helper node_to_owned_ref currently strips the
leading format byte by slicing bytes[1..], which hardcodes framing; update it to
use the same unpack logic as production by calling wacore_binary::util::unpack
(or the equivalent helper used by decrypt_frame) on the marshalled bytes and
pass the returned payload into OwnedNodeRef::new so tests mirror production
unpack/compression handling; locate node_to_owned_ref and replace the bytes[1..]
slicing with the unpack call and use its output for OwnedNodeRef::new.

In `@src/handlers/basic.rs`:
- Around line 84-91: The current handler calls OwnedNodeRef::to_owned_node(),
which clones the entire node and defeats the allocation savings—change
Client::handle_ack_response to accept a borrowed &wacore_binary::NodeRef<'_> and
call client.handle_ack_response(node.get()). Specifically, update the signature
of Client::handle_ack_response to take &wacore_binary::NodeRef<'_> and move the
ack-parsing logic into that borrowed form, then replace the to_owned_node() call
in handle with passing node.get() so no full clone is performed.

In `@src/handlers/receipt.rs`:
- Around line 22-30: The current call creates a new Owned Node and Arc every
receipt: Arc::new(node.to_owned_node()) inside handle should be replaced by
updating Client::handle_receipt's signature to avoid that allocation; change
Client::handle_receipt to accept either a reference (&Node or &OwnedNodeRef) if
it only needs read access, then call it with node.as_ref() or &*node (no
allocation), or change it to accept Arc<OwnedNodeRef> and call it with
Arc::clone(&node) to reuse the existing Arc; update the Client::handle_receipt
definition and all callers (and their imports) accordingly to match the new
parameter type.

In `@wacore/binary/src/jid.rs`:
- Around line 627-631: The branch that returns Err(JidError::InvalidFormat(...))
is constructing an error string that already includes the "Invalid JID format:"
prefix, causing duplication because JidError's Display impl adds that prefix;
change the returned InvalidFormat payload to just the server identifier or a
message without the "Invalid JID format:" prefix (e.g., server.to_string() or a
concise explanation like "unknown server '{server}'") so that
JidError::InvalidFormat and its Display produce a single prefixed message;
locate the return in the conditional around user_part.is_empty() and
Server::try_from(server).
- Around line 385-390: Call sites passing a &str to Jid::new must be updated to
supply a Server instance instead; find all uses of Jid::new(..., "...") and
replace the string literal with a Server constructed from that string (e.g.
Server::from("example.com"), Server::new("example.com"), or
Server::try_from("example.com").unwrap(), depending on which constructor/trait
the Server type exposes), so that every Jid::new invocation provides a Server
rather than a &str.

---

Outside diff comments:
In `@src/client.rs`:
- Around line 96-114: In resolve_waiters: avoid calling node.to_owned_node() for
every matching waiter; create a single Arc-owned payload (e.g., let owned =
Arc::new(node.to_owned_node())) the first time you hit a matching waiter and
then send Arc::clone(&owned) to each subsequent waiter (use Arc::clone or
owned.clone()) instead of Arc::new(node.to_owned_node()); this keeps the
existing swap_remove/counter.fetch_sub logic but replaces per-waiter deep clones
with cheap Arc clones to broadcast the same owned payload.

In `@wacore/binary/src/decoder.rs`:
- Around line 109-184: Tests are missing for the new server-decoding branches;
add table-driven unit tests that exercise malformed and valid-server paths for
read_jid_pair, read_ad_jid, read_interop_jid, and read_fb_jid. For each function
create inputs that produce: successful JidRef (when server string matches
expected), InvalidNode (when missing/empty user or wrong interop/messenger
server), and AttrParse (for read_jid_pair when server string fails
Server::try_from), assert the returned Result matches BinaryError::InvalidNode
or BinaryError::AttrParse or the expected JidRef fields; reference the decoder
methods (read_jid_pair, read_ad_jid, read_interop_jid, read_fb_jid) and
constants crate::jid::INTEROP_SERVER and crate::jid::MESSENGER_SERVER to build
the test cases so regressions in server-decoding are caught.

---

Duplicate comments:
In `@src/handlers/ib.rs`:
- Around line 29-37: The handler currently clones the Node with
node.to_owned_node(), which allocates per stanza; instead change
handle_ib_impl's signature to take a borrowed OwnedNodeRef (e.g.,
&wacore_binary::OwnedNodeRef) and call it with a reference to the existing
Arc-held node (for example handle_ib_impl(client, &*node).await or
handle_ib_impl(client, node.as_ref()).await) so no full Node is re-materialized;
update the handle_ib_impl function name/signature and all its call sites
accordingly (replace uses expecting owned Node with borrowed accessors).

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/client.rs`:
- Around line 1655-1662: The IQ response path is forcing a deep copy via
node.to_owned_node() before sending to waiters; change the waiter payload type
and send path so we forward a zero-copy OwnedNodeRef (or Arc<OwnedNodeRef>)
instead of an OwnedNode, avoiding to_owned_node(). Update the response_waiters
map entry type and any waiter.send calls (the waiter receiver and its creation
sites) to accept OwnedNodeRef or Arc<OwnedNodeRef>, and when dispatching for IQ
(where nr.tag == "iq" and id is present) send the existing node reference (or
wrap it in Arc) rather than calling node.to_owned_node(); ensure constructors of
waiters use the new type so decode remains zero-copy through request/response
handling.
- Around line 81-87: The matches method currently only checks attr.as_str(),
which misses JID-backed attributes; update the attribute comparison inside
matches (the closure using node.get_attr(...).and_then(|attr| attr.as_str())...)
to accept JID-backed values too by extracting the attribute as either a plain
string or a JID and comparing their string forms (e.g., try attr.as_str() first,
then attr.as_jid().map(|j| j.to_string()) or equivalent) against the filter
value; keep the outer logic and use the same attribute key lookup on NodeRef so
NodeFilter::from_jid and sent-node waiters will match JID-backed attrs.

In `@src/handlers/chatstate.rs`:
- Around line 56-63: The current fix resolves the type mismatch by calling
node.to_owned_node() before ChatstateStanza::parse, but that allocates and loses
zero-copy benefits; change ChatstateStanza::parse to take a borrowed node
reference (e.g., &wacore_binary::NodeRef<'_> or
&wacore_binary::OwnedNodeRef::NodeRef view) so you can pass a reference directly
from the Arc<wacore_binary::OwnedNodeRef> without calling to_owned_node(),
update the parse signature and all callers (including handle) to accept the
borrowed reference, and remove the to_owned_node() allocation where possible to
preserve zero-copy semantics.

In `@src/handlers/notification.rs`:
- Around line 37-45: The handler currently calls node.to_owned_node(), which
deep-clones the stanza and defeats zero-copy; instead, stop rematerializing and
call handle_notification_impl with a reference to the existing node (e.g., pass
&*node or node.as_ref()) and change handle_notification_impl (and any helpers it
calls) to accept borrowed node types (prefer wacore_binary::NodeRef or
&wacore_binary::OwnedNodeRef) rather than an owned cloned node; update all call
sites of handle_notification_impl and its helpers to the new signature so they
operate on NodeRef/OwnedNodeRef references and preserve the zero-copy decode
path.

In `@src/message.rs`:
- Around line 3585-3586: The test code currently masks failures by using
unwrap_or(Server::Lid) after wacore_binary::jid::Server::try_from(lid_server);
replace each unwrap_or(...) with expect(...) on the Result returned by try_from
so a mapping failure is explicit (e.g., change
wacore_binary::jid::Server::try_from(lid_server).unwrap_or(wacore_binary::jid::Server::Lid)
to wacore_binary::jid::Server::try_from(lid_server).expect("unexpected Server
mapping for lid_server") ), and make the same replacement for the other
identical occurrences of try_from(...).unwrap_or(...) in this file so tests fail
visibly on bad mappings.

In `@wacore/benches/send_receive_benchmark.rs`:
- Around line 190-193: The benchmark fixture silently falls back on Server::Pn
due to unwrap_or_default; change the call in the Jid::new construction to fail
fast by replacing unwrap_or_default() with expect(...) so invalid server strings
cause an immediate panic; specifically update the
wacore_binary::jid::Server::try_from(server) usage inside the Jid::new call to
use expect with a clear message (e.g., "invalid server in benchmark fixture") so
Jid::new and Server::try_from failures surface during setup.

---

Duplicate comments:
In `@src/client.rs`:
- Around line 4885-4892: The test helper node_to_owned_ref currently slices off
the first byte (bytes[1..]) which hardcodes the framing and diverges from
production unpacking; change it to call the same unpacking used in decrypt_frame
(e.g., util::unpack or the project’s unpack function) on the marshalled bytes
returned by wacore_binary::marshal::marshal and then pass the unpacked/raw
protocol bytes into wacore_binary::OwnedNodeRef::new so tests mirror production
framing behavior (keep the existing expect messages).
- Around line 3182-3184: The code currently clones every incoming Node via
node.to_owned() before calling pair::handle_iq, causing unnecessary deep copies
for non-pair IQs; fix this by either (A) adding a cheap, non-allocating guard
that inspects the NodeRef (e.g. check stanza type, tag/child name or an
attribute that identifies pairing IQs) and only call node.to_owned() when that
guard indicates a pairing IQ, or (B) update pair::handle_iq to accept a
NodeRef/OwnedNodeRef (e.g. &NodeRef) so you can call pair::handle_iq(self,
&node).await without cloning; locate the use of node.to_owned() and
pair::handle_iq in the client.rs snippet and apply one of these changes to avoid
the unnecessary deep copy.

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/client.rs`:
- Around line 3624-3637: build_ack_node currently calls to_string_cow() when
constructing NodeValue for id, from, participant and type which forces
JID-backed attributes through stringification and extra allocations; instead,
preserve the typed/borrowed attribute values when creating NodeValue (avoid
to_string_cow()) by using the attribute value directly from node.get_attr(...)
or the NodeRef API that returns a borrowed/typed representation so NodeValue is
built without intermediate String allocation (update the constructions for id,
from, participant and the type branch in build_ack_node to use the
direct/borrowed attr values).

In `@src/handlers/ib.rs`:
- Around line 29-36: The code currently calls node.to_owned_node() in handle
which unnecessarily allocates the full tree; change handle to pass a borrowed
reference instead by using node.get() or &NodeRef<'_> and update handle_ib_impl
signature to accept a borrowed NodeRef (or &NodeRef) so it reads from the
borrowed data without allocation; ensure call sites using handle_ib_impl (and
its async signature) are adjusted to accept the borrowed type and that lifetimes
compile.

---

Duplicate comments:
In `@src/client.rs`:
- Around line 4886-4893: The test helper node_to_owned_ref currently strips the
leading format byte by slicing bytes[1..] which diverges from production;
instead call wacore_binary::util::unpack on the marshaled bytes (the same
contract decrypt_frame uses) and pass the unpacked payload into
wacore_binary::OwnedNodeRef::new, propagating or expect()-ing the Result as
appropriate so tests exercise the same framing/unpacking logic as production.
- Around line 1656-1663: The code currently calls node.to_owned_node() when
delivering IQ responses which forces allocation; change the response_waiters
storage and send path to carry Arc<OwnedNodeRef> instead of Node so delivery is
zero-copy: update the type stored in self.response_waiters to Arc<OwnedNodeRef>,
stop calling node.to_owned_node() here, send an Arc clone (e.g.
waiter.send(arc_node.clone())), and defer calling to_owned_node() only in the
final consumer that actually needs an owned Node; adjust any related type
signatures (response_waiters map value, waiter channel type, and consumer
handlers) to accept Arc<OwnedNodeRef>.
- Around line 3183-3185: The code clones every incoming IQ via node.to_owned()
before dispatching to pair::handle_iq, causing unnecessary copies; either change
pair::handle_iq signature to accept a borrow (e.g., &Node or &Stanza) so you can
pass &node directly, or add a cheap pre-check (inspect
node.tag()/attrs/type/ping marker) and only call node.to_owned() when the stanza
is actually pair-related; update all call sites and types in pair::handle_iq
(and its helpers) accordingly to avoid the unconditional deep clone.

In `@src/handlers/basic.rs`:
- Around line 84-93: The handler currently clones the node via to_owned_node()
and passes an OwnedNodeRef into Client::handle_ack_response, losing zero-copy
benefits; change the handler in handle(...) to call client.handle_ack_response
with a borrowed NodeRef by passing node.get() (i.e., a
&wacore_binary::NodeRef<'_>) instead of owned_node, and update
Client::handle_ack_response signature (and its callers) to accept self:
&Arc<Self> and node: &wacore_binary::NodeRef<'_> so the ack parsing logic works
with a borrowed NodeRef without cloning.

In `@src/handlers/chatstate.rs`:
- Around line 56-63: The handler calls node.to_owned_node() and then
ChatstateStanza::parse(&owned), forcing an owned clone; update the parser to
accept a borrowed node reference (e.g., change ChatstateStanza::parse signature
to take &NodeRef<'_>) and call it with the existing node reference instead of
creating an OwnedNodeRef (remove the to_owned_node() usage in the handle
method). Ensure ChatstateStanza::parse and any downstream uses are updated to
work with &NodeRef<'_> so chatstate handling remains zero-copy end-to-end.

In `@src/handlers/notification.rs`:
- Around line 37-44: The handler currently calls node.to_owned_node(), which
defeats zero-copy by rematerializing the stanza; instead, change async fn handle
to pass a reference to the existing OwnedNodeRef/NodeRef (e.g.,
&Arc<wacore_binary::OwnedNodeRef> or &wacore_binary::NodeRef) into the
processing path and move handle_notification_impl and its helper functions as
methods or free functions that accept &OwnedNodeRef / &NodeRef (or implement
them as impl blocks on OwnedNodeRef/NodeRef). Replace the to_owned_node() call
in handle with a direct call into the new OwnedNodeRef/NodeRef-based method
(e.g., node.handle_notification_impl(...)) and update all helper usages to
operate on the reference types so no cloning/rematerialization occurs at the
boundary.

In `@src/handlers/receipt.rs`:
- Around line 22-29: The handler is cloning the entire node tree with
node.to_owned_node() on every receipt; change Client::handle_receipt to accept a
ref-counted node (Arc<wacore_binary::OwnedNodeRef>) or a borrowed NodeRef (e.g.,
&wacore_binary::NodeRef) so you can pass the node directly without cloning;
update the Client::handle_receipt signature and all its callers/implementations
accordingly, and in receipt.rs call client.handle_receipt(node.clone()).await
(or pass &*node if you choose &NodeRef) instead of
Arc::new(node.to_owned_node()) to preserve zero-copy and avoid the tree copy.

In `@src/message.rs`:
- Around line 3585-3586: Replace the silent fallback from unwrap_or(Server::Lid)
with expect so tests fail loudly when the mapping is broken: locate each use of
wacore_binary::jid::Server::try_from(lid_server).unwrap_or(wacore_binary::jid::Server::Lid)
(e.g., in src/message.rs at the instances around the try_from calls you've
changed) and change to call expect(...) on the Result, providing a clear message
like "unexpected Server mapping for lid_server: {lid_server}" so a bad mapping
panics instead of silently using Server::Lid; apply the same replacement to the
other occurrences noted (around the other try_from sites).

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/client/device_registry.rs`:
- Around line 240-241: The cleanup currently cross-products user aliases by
calling all_keys() and iterating both Server::Lid and Server::Pn, which throws
away the LID/PN distinction from resolve_lookup_keys() and can purge unrelated
accounts; update delete_sessions_for_devices() (and the similar block at
303-310) to match on resolve_lookup_keys() (UserLookupKeys::LidWithPn,
::PnWithLid, ::Unknown) and only call clear_device_record()/purge for the
specific (user, server) pairs returned (use the caller's server for the Unknown
case) rather than iterating both server variants or using all_keys(). Ensure
clear_device_record() is invoked with the exact (user, server) tuple from the
match so you preserve the original alias semantics.

In `@src/features/media_reupload.rs`:
- Around line 110-112: The code currently calls
notification_node.to_owned_node() which deep-clones the node and payload;
instead keep the borrowed path by adding an overload that accepts a borrowed
node reference (e.g., parse_media_retry_notification_borrowed or change
parse_media_retry_notification to accept &Node/NodeRef) and call that here with
the existing notification_node (avoid to_owned_node()); update the
parse_media_retry_notification implementation (or add a small wrapper) to
operate only on the borrowed stanza without requiring ownership so no deep clone
occurs.

In `@src/handlers/ib.rs`:
- Around line 113-116: The nested pattern matching around extracting
routing_info can be simplified with a let-chain: replace the two-level `if let`
(the call to `child.get_optional_child("routing_info")` binding
`routing_info_node` and the inner `if let
Some(NodeContentRef::Bytes(routing_bytes)) =
routing_info_node.content.as_deref()`) with a single `if let
Some(routing_info_node) = child.get_optional_child("routing_info") && let
Some(NodeContentRef::Bytes(routing_bytes)) =
routing_info_node.content.as_deref() { ... }` so that routing_info_node and
routing_bytes are bound in one condition (update the block body to use these
bindings).

In `@src/handlers/receipt.rs`:
- Line 28: The call currently does Arc::clone(&node) before
client.handle_receipt which unnecessarily bumps the Arc refcount; instead move
node into the call (client.handle_receipt(node).await) and remove the Arc::clone
usage. Update the handle_receipt signature if needed so it accepts the owned
Arc<Node> (by value) rather than borrowing, and ensure there are no subsequent
uses of node after this call so the move is valid.

In `@src/retry.rs`:
- Around line 57-64: The retry path currently materializes an owned Node inside
handle_retry_receipt (via node.to_owned_node()), which forces copying of the
retry stanza; change handle_retry_receipt to accept and work with a borrowed
node reference (e.g. &NodeRef<'_> or &OwnedNodeRef) and parse directly from
node.get() instead of calling to_owned_node(), and update/move any helper
functions that currently expect owned nodes to accept the borrowed NodeRef so no
allocation occurs when handling Receipt in handle_retry_receipt.

In `@wacore/binary/src/decoder.rs`:
- Around line 109-115: Add a regression test that exercises the new fail-fast
path in read_jid_pair: craft a minimal binary buffer representing a JID_PAIR
where the server string is invalid, feed it to the decoder (the same decoding
entry used in tests that ultimately calls read_jid_pair) and assert the decode
returns Err(BinaryError::AttrParse(..)); this ensures Server::try_from(...) in
read_jid_pair fails and the AttrParse error is produced rather than falling back
to the old Server::Pn behavior. Include references to read_jid_pair,
Server::try_from, and BinaryError::AttrParse in the test assertion so it fails
if the old fallback returns a valid Server instead of an error.

In `@wacore/src/iq/chatstate.rs`:
- Around line 140-145: The parse() path using attrs.optional_jid("from") treats
both missing and malformed JIDs the same because optional_jid() records parse
failures in attrs.errors and returns None, making
ChatstateParseError::InvalidJid unreachable; fix by distinguishing malformed vs
missing JID: after calling let mut attrs = node.attr_parser(); use either a
required-parsing helper (e.g., call attrs.jid("from") or similar) that returns a
parse error you can map to ChatstateParseError::InvalidJid, or, if you keep
optional_jid("from"), immediately inspect attrs.errors before returning
MissingFrom and return ChatstateParseError::InvalidJid when a JID parse error
for "from" is present (ensuring SelfEcho logic still checks
attrs.optional_jid("to") as before).

---

Outside diff comments:
In `@src/client/lid_pn.rs`:
- Around line 139-150: The code compares target.server to string constants
DEFAULT_USER_SERVER and HIDDEN_USER_SERVER; instead, use the typed Server enum
variants for consistency—replace checks against
wacore_binary::jid::DEFAULT_USER_SERVER and
wacore_binary::jid::HIDDEN_USER_SERVER with comparisons to
wacore_binary::jid::Server::Pn and wacore_binary::jid::Server::Lid respectively
(keeping the existing Jid construction that uses Server::Lid), and adjust any
imports or fully-qualify Server where needed so all server equality checks
consistently use the Server enum.

In `@src/handlers/ib.rs`:
- Around line 128-133: The direct mutation using
client_clone.persistence_manager.modify_device(...) to set
device.edge_routing_info = Some(routing_bytes) must be replaced by constructing
an appropriate DeviceCommand (e.g., SetEdgeRouting or similar) carrying
routing_bytes and sending it to
client_clone.persistence_manager.process_command(command). Also update reads to
use client_clone.persistence_manager.get_device_snapshot() where needed; ensure
you remove the direct modify_device call and use the
PersistenceManager::process_command() path so state changes go through
centralized command handling.

In `@src/message.rs`:
- Around line 73-90: The code silently returns when plaintext_node.content
exists but isn't NodeContentRef::Bytes; update the handling in the plaintext
block surrounding plaintext_node and wa::Message::decode so that if
plaintext_node.content.as_deref() is Some but not Bytes you emit a warning
(e.g., using log::warn!) that includes info.id and a brief description that
plaintext content was present but not bytes before returning, while keeping the
existing decode/dispatch_parsed_message flow for the Bytes case.
- Around line 366-372: The code repeatedly calls
self.custom_enc_handlers.read().await inside the per-<enc> node decode loop
which reacquires the async RwLock on every node; hoist the read guard outside
the inner node loop by calling let guard = self.custom_enc_handlers.read().await
once (or at the start of the multi-enc stanza), use
guard.get(enc_type.as_ref()).cloned() to fetch the handler, then drop the guard
(or let it go out of scope) before any subsequent await in the loop so the async
lock isn't held across awaits and isn't re-acquired per node; refer to
custom_enc_handlers, enc_type, and the handler variable when making this change.

---

Duplicate comments:
In `@src/client.rs`:
- Around line 1660-1667: The current code rematerializes the full stanza with
node.to_owned_node() before sending to the waiter, causing a deep copy on every
waiter hit; change the response_waiters map to hold OwnedNodeRef (or
Arc<OwnedNodeRef>) instead of OwnedNode and update all insertion sites to store
an OwnedNodeRef/Arc there so that in this block you simply clone/clone the Arc
and send that (avoid calling node.to_owned_node()); adjust uses of
response_waiters.lock().await.remove(id) and waiter.send(...) to accept/send the
OwnedNodeRef/Arc type and update any types/traits (e.g., sender type)
accordingly.
- Around line 3188-3190: The code currently always clones the stanza via
node.to_owned() before calling pair::handle_iq, causing unnecessary copies;
either change pair::handle_iq to accept a non-owning reference type (e.g.,
NodeRef or OwnedNodeRef) so it can operate without forcing a deep clone, or add
a cheap pre-check (inspect node.tag/name/attrs on the NodeRef) right before the
call and only call node.to_owned() when the stanza matches the pairing criteria;
update the pair::handle_iq signature and call sites (or add the predicate check)
accordingly so non-pair IQs avoid the allocation.
- Around line 3628-3641: The build_ack_node function currently forces JID-backed
attributes (id, from, participant, and the type attr path) through
to_string_cow() then reconstructs NodeValue, causing extra formatting/allocation
on the ACK hot path; replace those round-trips by using the value-preserving
conversion from wacore_binary's ValueRef (or the crate's direct impl
From<ValueRef> for NodeValue / NodeValue::Jid helper) so you construct NodeValue
directly from the attribute ValueRef (e.g., change map(|v|
NodeValue::from(v.to_string_cow().as_ref())) to the direct conversion like
NodeValue::from(v) or the crate-specific ValueRef→NodeValue API) for id, from,
participant and the type branch, preserving typed values and avoiding string
allocations.
- Around line 4890-4896: The test helper node_to_owned_ref currently hardcodes
framing by calling wacore_binary::marshal and slicing bytes[1..]; change it to
use wacore_binary::marshal_auto(&node) and then pass the result through
wacore_binary::util::unpack(...) to obtain the raw protocol payload, and then
construct Arc::new(wacore_binary::OwnedNodeRef::new(unpacked).expect(...)). This
ensures node_to_owned_ref follows the production unpack contract and will stay
correct if framing/compression changes.

In `@src/handlers/notification.rs`:
- Around line 206-211: Several handlers currently call to_owned_node() which
forces allocations and prevents zero-copy; update the parsing/handler APIs to
accept a NodeRef<'_> (borrowed) and then pass &node (the existing NodeRef)
instead of calling to_owned_node() in call sites such as
crate::pair_code::handle_pair_code_notification, DeviceNotification::try_parse,
BusinessNotification::try_parse, and GroupNotification::try_from_node; change
those function signatures to take &NodeRef<'_> (or &impl AsNodeRef) and update
their internals to work with the borrowed node so you can remove the
to_owned_node() conversions across these notification handlers, tracking any
remaining unported call sites as follow-up work.

In `@src/message.rs`:
- Around line 3598-3599: The test code currently masks failures by using
unwrap_or(wacore_binary::jid::Server::Lid) after
wacore_binary::jid::Server::try_from(lid_server); replace those fallbacks with
expect(...) so the conversion failure fails loudly and surfaces regressions —
update the expression where Server::try_from(lid_server) is called (and the
other identical occurrences in this file) to call expect with a clear message
indicating the failed lid_server → Server mapping instead of defaulting to
Server::Lid.